Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new loot dialog options: rare items #23

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vn971
Copy link

@vn971 vn971 commented Jan 15, 2017

The idea is to allow showing dialog/notification for rare/quest items,
while still not showing any notifications otherwise.

Notification logic has been re-factored a bit.

The idea is to allow showing dialog/notification for rare/quest items,
while still not showing any notifications otherwise.

Notification logic has been re-factored a bit.
@vn971
Copy link
Author

vn971 commented Jan 15, 2017

UPD: offtopic deleted.

@Rijackson
Copy link

It would be better to post the bug reports on the Andor's Trail forums.

Stoutford is in development, and at this point nothing should be taken as much more than a placeholder. The gloves and shoes are not as powerful as you think: note the actor condition. The stats you see now are also not necessarily final (in fact, it's quite likely they will change). There is a quest in Stoutford, as well as three other new ones in the stoutford_tests branch that can be completed.

@vn971
Copy link
Author

vn971 commented Jan 15, 2017

To reviewers. Note that the old code had some strange parts I'm not sure I reflected 1-to-1 in the new code.

For example, there was intersection between "show notification" and "show dialog" events. And one did not fully include the other. The logic was also split across files.
In the new code, "show notification" code always says true whenever "show dialog" says true.

@@ -6,7 +6,7 @@

public final class ItemType {

public static enum DisplayType {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove the "static" modifier ?

Copy link
Author

@vn971 vn971 Jan 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's just no need for that, since inner enums are static anyway. http://stackoverflow.com/questions/663834/in-java-are-enum-types-inside-a-class-static
But I guess if that's a common code style in AT, I could revert that part.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Zukero should I revert that part?

@vn971
Copy link
Author

vn971 commented Jan 17, 2017

@Rijackson thank you for your answer. You may be right, I didn't test Stoudford throughly. You probably have to have some other quests completed at the time you enter Stoudford, or maybe I didn't click/notice something (yet).

@Rijackson
Copy link

There are no specific requirements to start the new quest in Stoutford.

Which boss monster do you mean in Flagstone? The Winged Demon in the last map?

@vn971
Copy link
Author

vn971 commented Jan 19, 2017

@Rijackson I'll have to re-check. It seemed like a village where inside each house there is either nobody inside, or just a friendly man who's not talking much. Apart from the priest who is just angry. Again, I'll re-check and try to talk to everyone yet again.

About Flagstone - yes, that's the one (the second, last demon). After the demon there is only the villain inside (to whom you can talk). Also note I have the "optimized drawing" checked in the settings.

@Zukero
Copy link
Owner

Zukero commented Jan 19, 2017 via email

@Zukero
Copy link
Owner

Zukero commented Jan 19, 2017 via email

@vn971
Copy link
Author

vn971 commented Jan 21, 2017

EDIT: deleted offtopic question.

@Zukero
Copy link
Owner

Zukero commented Jan 21, 2017 via email

@vn971
Copy link
Author

vn971 commented Apr 6, 2018

I see that Andor's Trail is still alive, yet this PR is kinda stale.
Why not merge it? (Besides it being currently un-mergeable, but that can be fixed I guess.)

@Zukero
Copy link
Owner

Zukero commented Apr 6, 2018

Because it was opened concurrently to a huge UI overhaul and that it is currently unmergeable :)
That UI overhaul was recently merged, so if you can find the time to update your PR, I'll be happy to merge it.

@vn971
Copy link
Author

vn971 commented Apr 9, 2018 via email

@Zukero
Copy link
Owner

Zukero commented Apr 9, 2018

No problem. Thanks for submitting this in the first place. I'll try to find time to fix it myself.

Chriz76 pushed a commit to Chriz76/andors-trail that referenced this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants